THREESCALE-11498: API plans features #3949

jlledom · 2024-11-28T16:52:28Z

What this PR does / why we need it:

The plan features API is a bit messy. For instance, AFAIK there's no endpoints to create or remove features for a plan, they must be created and deleted from the UI. The API only allows enabling or disabling features previously created from the UI. These are the endpoints:

GET /admin/api/account_plans/:account_plan_id/features(.:format)
- Returns the enabled features
POST /admin/api/account_plans/:account_plan_id/features(.:format)
- Enables a feature
- It must receive feature_id as query param or form data
- Returns 200 on success
- Returns 409 when the feature was already enabled
- Returns 404 when the plan doesn't exist or the plan exists but the feature doesn't exists for that plan
DELETE /admin/api/account_plans/:account_plan_id/features/:id(.:format)
- Disables a feature
- Takes the feature ID from the path, not in parameters like in the enabling endpoint
- 200 on success
- 404 when the endpoint was already disabled
- 404 when the plan doesn't exists or the feature doesn't exist for the plan

I find this inconsistent and confusing. I guess we need to keep it as it is to avoid breaking changes, but it'd be good if we create a new API for this, at some point... in the future...

In this PR I'm making a few changes:

I renamed a scope and rewrote some comments to make clear in the code which association returns all available features and which one only returns the enabled ones: a5d6b40
I fixed the Open API spec for Account Plans Features endpoints. It was wrong and it included a new endpoint which doesn't exist: POST /admin/api/account_plans/:account_plan_id/features/:id(.:format)
I modified to be POST /admin/api/account_plans/:account_plan_id/features which exists and it's the correct one to enable features. d1b1d00
I also renamed the same endpoints for Service and Application plans, to explicitly mention enabling and disabling: 60739a7

Which issue(s) this PR fixes

https://issues.redhat.com/browse/THREESCALE-11498

previous name was hard to understand, and incorrect, since features may belong to service or account plans

mayorova · 2024-12-12T14:55:59Z

doc/active_docs/account_management_api.json

      "post": {
-        "summary": "Account Plan Features Create",
-        "description": "Associate an account feature to an account plan.",
+        "summary": "Account Plan Features Enable",


Suggested change

"summary": "Account Plan Features Enable",

"summary": "Account Plan Feature Enable",

Maybe this to clarify further that it is for a single feature.

mayorova · 2024-12-12T14:57:20Z

doc/active_docs/account_management_api.json

      "delete": {
-        "summary": "Account Plan Features Delete",
-        "description": "Deletes the association of an account feature to an account plan.",
+        "summary": "Account Plan Features Disable",


Suggested change

"summary": "Account Plan Features Disable",

"summary": "Account Plan Feature Disable",

mayorova · 2024-12-12T15:03:52Z

The plan features API is a bit messy. For instance, AFAIK there's no endpoints to create or remove features for a plan, they must be created and deleted from the UI. The API only allows enabling or disabling features previously created from the UI.

I believe this is not accurate.

There is actually a set of endpoints under "Service Features" section that allows managing application plans features (creating new ones, updating, deleting etc.)

I'm not sure about features for service and account plans though...

mayorova · 2024-12-12T15:11:06Z

OK, so for account plan features it seems that there are also endpoints for CRUD:

But I can't find the same for the service plan features 😬

jlledom · 2024-12-18T12:31:42Z

@mayorova I applied your suggestions 6afe150

There is actually a set of endpoints under "Service Features" section that allows managing application plans features
OK, so for account plan features it seems that there are also endpoints for CRUD:

So, service features are created and enabled by:

/admin/api/services/{service_id}/features.xml
/admin/api/service_plans/{service_plan_id}/features.xml

Account features are created and enabled by:

/admin/api/features.xml
/admin/api/account_plans/{account_plan_id}/features.xml

Application features can only be created from UI and enabled by:
/admin/api/application_plans/{application_plan_id}/features.xml

I love porta consistency.

akostadinov · 2024-12-18T12:40:30Z

app/models/plan.rb

-  has_many :features, :through => :features_plans do
-    # returns all features owned by issuer, not only enabled by plan
-    def of_service
+  has_many :features_plans, as: :plan, dependent: :delete_all


why did you add dependent: :delete_all?

I don't remember, but it was a good reason, for sure!

It makes sense, though. features_plans is a N-N table between features and plans, and every record represents a feature enabled for a plan. It makes no sense to keep a feature enabled for a plan after the plan is removed.

how many of them can there be? We split deleting for transactions failing reason when they are too big. That's why we have the background deletion.

The plan having more features has 88. I don't think that's usual though, probably most of them have less than 10. Also, the association between features and features_plans is marked as dependent: :delete_all as well:

porta/app/models/feature.rb

Line 17 in 908be5d

has_many :features_plans, :dependent => :delete_all

mayorova · 2024-12-18T12:52:27Z

So, service features are created and enabled by:

/admin/api/services/{service_id}/features.xml /admin/api/service_plans/{service_plan_id}/features.xml

Account features are created and enabled by:

/admin/api/features.xml /admin/api/account_plans/{account_plan_id}/features.xml

Application features can only be created from UI and enabled by: /admin/api/application_plans/{application_plan_id}/features.xml

I love porta consistency.

Not exactly 😅

/admin/api/services/{service_id}/features.xml is for managing the Application plan features.

It's endpoints for Service plans features CRUD that are missing, and can only be done via UI.

jlledom · 2024-12-18T12:56:22Z

So, service features are created and enabled by:
/admin/api/services/{service_id}/features.xml /admin/api/service_plans/{service_plan_id}/features.xml
Account features are created and enabled by:
/admin/api/features.xml /admin/api/account_plans/{account_plan_id}/features.xml
Application features can only be created from UI and enabled by: /admin/api/application_plans/{application_plan_id}/features.xml
I love porta consistency.

Not exactly 😅

/admin/api/services/{service_id}/features.xml is for managing the Application plan features.

It's endpoints for Service plans features CRUD that are missing, and can only be done via UI.

Ah, I was so wrong, so the application features are created through the services endpoint, and there's no endpoint for services. Nice, much better! 😅

Rename of_service scope to all_available

a5d6b40

previous name was hard to understand, and incorrect, since features may belong to service or account plans

jlledom self-assigned this Nov 28, 2024

jlledom force-pushed the THREESCALE-11498-api-features-plans branch from 1c4bdef to a5d6b40 Compare November 29, 2024 09:19

jlledom changed the title ~~Threescale 11498 api features plans~~ THREESCALE-11498: API plans features Nov 29, 2024

jlledom added 2 commits November 29, 2024 11:18

Fix API Docs to represent the actual API

d1b1d00

OAS Spec: Improve endpoint descriptions

60739a7

jlledom marked this pull request as ready for review November 29, 2024 11:58

jlledom requested review from mayorova, akostadinov and lvillen November 29, 2024 11:58

mayorova reviewed Dec 12, 2024

View reviewed changes

mayorova previously approved these changes Dec 12, 2024

View reviewed changes

Endpoint descriptions: Feature in singular

6afe150

jlledom dismissed mayorova’s stale review via 6afe150 December 18, 2024 12:22

akostadinov reviewed Dec 18, 2024

View reviewed changes

mayorova approved these changes Dec 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

THREESCALE-11498: API plans features #3949

THREESCALE-11498: API plans features #3949

jlledom commented Nov 28, 2024 •

edited

Loading

mayorova Dec 12, 2024

mayorova Dec 12, 2024

mayorova commented Dec 12, 2024

mayorova commented Dec 12, 2024

jlledom commented Dec 18, 2024

akostadinov Dec 18, 2024

jlledom Dec 18, 2024

akostadinov Dec 18, 2024 •

edited

Loading

jlledom Dec 19, 2024

mayorova commented Dec 18, 2024

jlledom commented Dec 18, 2024

	"summary": "Account Plan Features Enable",
	"summary": "Account Plan Feature Enable",

	"summary": "Account Plan Features Disable",
	"summary": "Account Plan Feature Disable",

THREESCALE-11498: API plans features #3949

Are you sure you want to change the base?

THREESCALE-11498: API plans features #3949

Conversation

jlledom commented Nov 28, 2024 • edited Loading

mayorova Dec 12, 2024

Choose a reason for hiding this comment

mayorova Dec 12, 2024

Choose a reason for hiding this comment

mayorova commented Dec 12, 2024

mayorova commented Dec 12, 2024

jlledom commented Dec 18, 2024

akostadinov Dec 18, 2024

Choose a reason for hiding this comment

jlledom Dec 18, 2024

Choose a reason for hiding this comment

akostadinov Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

jlledom Dec 19, 2024

Choose a reason for hiding this comment

mayorova commented Dec 18, 2024

jlledom commented Dec 18, 2024

jlledom commented Nov 28, 2024 •

edited

Loading

akostadinov Dec 18, 2024 •

edited

Loading